global sort: support setting cloud storage uri variable #46407

lichunzhu · 2023-08-24T14:34:53Z

What problem does this PR solve?

Issue Number: ref #45719

Problem Summary:

What is changed and how it works?

support setting global sort uri variable

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

mysql> set @@global.tidb_global_sort_uri="s3://tiflow-test?access-key=testid&secret-access-key=testkey8&endpoint=http://127.0.0.1:5000";
Query OK, 0 rows affected (0.05 sec)
mysql> select @@global.tidb_global_sort_uri;
+-------------------------------+
| @@global.tidb_global_sort_uri |
+-------------------------------+
| s3://tiflow-test/             |
+-------------------------------+
1 row in set (3.69 sec)

Side effects

None

Documentation

None

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

support setting tidb_global_sort_uri variable

tiprow · 2023-08-24T14:35:23Z

Hi @lichunzhu. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

codecov · 2023-08-24T14:54:55Z

Codecov Report

Merging #46407 (46f427e) into master (431d30f) will decrease coverage by 0.4975%.
Report is 19 commits behind head on master.
The diff coverage is 100.0000%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #46407        +/-   ##
================================================
- Coverage   73.2928%   72.7954%   -0.4975%     
================================================
  Files          1322       1346        +24     
  Lines        396399     403046      +6647     
================================================
+ Hits         290532     293399      +2867     
- Misses        87336      91069      +3733     
- Partials      18531      18578        +47

Flag	Coverage Δ
integration	`27.7906% <44.0000%> (?)`
unit	`73.4153% <100.0000%> (+0.1225%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`54.0444% <ø> (ø)`
parser	`84.9717% <ø> (+0.0107%)`	⬆️
br	`48.4508% <100.0000%> (-4.1552%)`	⬇️

tangenta · 2023-08-24T16:27:07Z

sessionctx/variable/sysvar.go

@@ -2227,6 +2227,17 @@ var defaultSysVars = []*SysVar{
 		DDLDiskQuota.Store(TidbOptUint64(val, DefTiDBDDLDiskQuota))
 		return nil
 	}},
+	{Scope: ScopeSession, Name: TiDBGlobalSortURI, Value: "", Type: TypeStr, Validation: func(sv *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) {


Only session scope? The usability could be bad because it will be reset to "" every time a new session is created.

Benjamin2037 · 2023-09-06T08:01:02Z

sessionctx/variable/sysvar.go

+		}
+		return normalizedValue, nil
+	}, GetSession: func(sv *SessionVars) (string, error) {
+		return sv.GlobalSortURI, nil


Do we need add mask function to hide ak sk information？

lichunzhu · 2023-09-07T12:44:56Z

@tangenta @Benjamin2037 PTAL again. I add a manual test in PR description

sessionctx/variable/tidb_vars.go

Benjamin2037

LGTM

lichunzhu · 2023-09-08T10:07:11Z

/retest

tiprow · 2023-09-08T10:07:45Z

@lichunzhu: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tangenta · 2023-09-08T10:47:45Z

/retest

tiprow · 2023-09-08T10:48:16Z

@tangenta: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lichunzhu · 2023-09-08T11:59:56Z

/retest

tiprow · 2023-09-08T12:00:37Z

@lichunzhu: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lichunzhu · 2023-09-08T12:06:08Z

/ok-to-test

hawkingrei · 2023-09-08T14:50:27Z

/test all

lichunzhu · 2023-09-09T07:00:35Z

/retest

wjhuang2016 · 2023-09-09T12:07:04Z

/retest

wjhuang2016 · 2023-09-09T12:15:12Z

/test unit-test

tiprow · 2023-09-09T12:15:26Z

@wjhuang2016: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test tiprow_fast_test

Use /test all to run all jobs.

In response to this:

/test unit-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

wjhuang2016 · 2023-09-10T19:00:57Z

/retest

lichunzhu · 2023-09-11T03:33:03Z

/retest

lichunzhu · 2023-09-11T06:13:46Z

/retest

D3Hunter · 2023-09-11T06:14:17Z

/retest

tangenta · 2023-09-11T06:55:47Z

sessionctx/variable/sysvar.go

@@ -26,6 +26,7 @@ import (
 	"time"

 	"github.com/pingcap/errors"
+	"github.com/pingcap/tidb/br/pkg/storage"


Maybe use the RedactURL in parser/ast/misc.go instead?

lichunzhu · 2023-09-11T08:58:36Z

/retest

lichunzhu · 2023-09-11T09:44:26Z

/retest

Benjamin2037 · 2023-09-11T10:17:26Z

/retest

support passing global sort uri

935f78b

lichunzhu requested a review from a team as a code owner August 24, 2023 14:34

ti-chi-bot bot added do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 24, 2023

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Aug 24, 2023

tangenta reviewed Aug 24, 2023

View reviewed changes

Benjamin2037 requested a review from wjhuang2016 September 6, 2023 07:20

Benjamin2037 reviewed Sep 6, 2023

View reviewed changes

tangenta mentioned this pull request Sep 7, 2023

ddl, lightning: integrate operator for global sort #46734

Merged

12 tasks

lichunzhu added 2 commits September 7, 2023 20:40

address comment

3dc1ed2

remove session variable

79d7a58

tangenta reviewed Sep 7, 2023

View reviewed changes

sessionctx/variable/tidb_vars.go Outdated Show resolved Hide resolved

lichunzhu added 3 commits September 7, 2023 23:42

fix bazel

c682be5

add more UT

7d62830

revise variable name

f66f177

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 8, 2023

fix comment

1c829d3

lichunzhu changed the title ~~global sort: support setting global sort uri variable~~ global sort: support setting cloud storage uri variable Sep 8, 2023

lichunzhu added 2 commits September 8, 2023 16:21

merge master and resolve conflicts

7d0b742

fix

2997f52

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 8, 2023

tangenta approved these changes Sep 8, 2023

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Sep 8, 2023

Benjamin2037 approved these changes Sep 8, 2023

View reviewed changes

resolve conflicts

261cb93

ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label Sep 8, 2023

revise variable name

bc4b91f

lichunzhu added 3 commits September 11, 2023 13:04

skip go leak

5c35ee9

skip go leak

315e965

fix

c019648

fix

e548319

tangenta reviewed Sep 11, 2023

View reviewed changes

lichunzhu added 2 commits September 11, 2023 15:13

fix

bb17064

try fix ut

46f427e

ti-chi-bot bot merged commit cbb7b0a into pingcap:master Sep 11, 2023
17 of 21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

global sort: support setting cloud storage uri variable #46407

global sort: support setting cloud storage uri variable #46407

lichunzhu commented Aug 24, 2023 •

edited

Loading

tiprow bot commented Aug 24, 2023

codecov bot commented Aug 24, 2023 •

edited

Loading

tangenta Aug 24, 2023

Benjamin2037 Sep 6, 2023

lichunzhu commented Sep 7, 2023

Benjamin2037 left a comment

lichunzhu commented Sep 8, 2023

tiprow bot commented Sep 8, 2023

tangenta commented Sep 8, 2023

tiprow bot commented Sep 8, 2023

lichunzhu commented Sep 8, 2023

tiprow bot commented Sep 8, 2023

lichunzhu commented Sep 8, 2023

hawkingrei commented Sep 8, 2023

lichunzhu commented Sep 9, 2023

wjhuang2016 commented Sep 9, 2023

wjhuang2016 commented Sep 9, 2023

tiprow bot commented Sep 9, 2023

wjhuang2016 commented Sep 10, 2023

lichunzhu commented Sep 11, 2023

lichunzhu commented Sep 11, 2023

D3Hunter commented Sep 11, 2023

tangenta Sep 11, 2023

lichunzhu commented Sep 11, 2023

lichunzhu commented Sep 11, 2023

Benjamin2037 commented Sep 11, 2023

global sort: support setting cloud storage uri variable #46407

global sort: support setting cloud storage uri variable #46407

Conversation

lichunzhu commented Aug 24, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

tiprow bot commented Aug 24, 2023

codecov bot commented Aug 24, 2023 • edited Loading

Codecov Report

tangenta Aug 24, 2023

Choose a reason for hiding this comment

Benjamin2037 Sep 6, 2023

Choose a reason for hiding this comment

lichunzhu commented Sep 7, 2023

Benjamin2037 left a comment

Choose a reason for hiding this comment

lichunzhu commented Sep 8, 2023

tiprow bot commented Sep 8, 2023

tangenta commented Sep 8, 2023

tiprow bot commented Sep 8, 2023

lichunzhu commented Sep 8, 2023

tiprow bot commented Sep 8, 2023

lichunzhu commented Sep 8, 2023

hawkingrei commented Sep 8, 2023

lichunzhu commented Sep 9, 2023

wjhuang2016 commented Sep 9, 2023

wjhuang2016 commented Sep 9, 2023

tiprow bot commented Sep 9, 2023

wjhuang2016 commented Sep 10, 2023

lichunzhu commented Sep 11, 2023

lichunzhu commented Sep 11, 2023

D3Hunter commented Sep 11, 2023

tangenta Sep 11, 2023

Choose a reason for hiding this comment

lichunzhu commented Sep 11, 2023

lichunzhu commented Sep 11, 2023

Benjamin2037 commented Sep 11, 2023

lichunzhu commented Aug 24, 2023 •

edited

Loading

codecov bot commented Aug 24, 2023 •

edited

Loading